refactor(relay): backend seams prep for local mode (PR 0/12) - #5506
refactor(relay): backend seams prep for local mode (PR 0/12)#5506loganj wants to merge 5 commits into
Conversation
Signed-off-by: npub13n66s06epmqf2kc3v373ez8hj65cuzyvxzjf93vwpervxqn2u7jq2qd9je <8cf5a83f590ec0955b11647d1c88f796a98e088c30a492c58e0e46c3026ae7a4@buzz.block.builderlab.xyz>
Signed-off-by: npub1z3hmzc9ryehxzedl5wzlvpyvja0d483peaja5zt6pd0209f9x2jspe2dxh <146fb160a3266e6165bfa385f6048c975eda9e21cf65da097a0b5ea7952532a5@buzz.block.builderlab.xyz>
Signed-off-by: npub1z3hmzc9ryehxzedl5wzlvpyvja0d483peaja5zt6pd0209f9x2jspe2dxh <146fb160a3266e6165bfa385f6048c975eda9e21cf65da097a0b5ea7952532a5@buzz.block.builderlab.xyz>
Signed-off-by: Brother Darryl <146fb160a3266e6165bfa385f6048c975eda9e21cf65da097a0b5ea7952532a5@buzz.block.builderlab.xyz>
Signed-off-by: Brother Darryl <146fb160a3266e6165bfa385f6048c975eda9e21cf65da097a0b5ea7952532a5@buzz.block.builderlab.xyz>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f29ecbdb0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| #[proc_macro_attribute] | ||
| pub fn enforce_sqlite_backend_declarations(_args: TokenStream, input: TokenStream) -> TokenStream { |
There was a problem hiding this comment.
enforce_sqlite_backend_declarations is a newly exported API but has no function-level documentation explaining its attribute syntax, generated inventory, or compile-time behavior. Add a /// comment to comply with the repository requirement that every new public API be documented.
AGENTS.md reference: AGENTS.md:L117-L117
Useful? React with 👍 / 👎.
| let key = path | ||
| .strip_prefix(base) | ||
| .unwrap() |
There was a problem hiding this comment.
The new non-test filesystem listing path calls unwrap() while processing directory entries, and the same function later introduces an expect() when constructing the continuation token. Replace these with normal error propagation through MediaError; production paths are explicitly prohibited from adding unwrap() or expect(), and a violated path invariant should fail the sweep rather than abort the process.
AGENTS.md reference: AGENTS.md:L116-L116
Useful? React with 👍 / 👎.
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Reviewed exact head 8f29ecbdb06ba042b1cc4d833d3d4d74b115fbf7 against stated base bbd20fae75ecc3bd7a83cc12a65379fac22a2b79.
Blocking findings
-
crates/buzz-media/src/storage.rs:452-455,498-499adds productionunwrap()andexpect()calls in the new filesystem listing path. This violates the repository's explicit rule inAGENTS.md:114-117: production paths must not introduceunwrap()/expect()and must use proper errors. The traversal and pagination logic make both invariants plausible, so this is not a claim of a reproduced panic; it is an explicit production-code policy violation. Replace both calls with propagatedMediaError/I/O errors, and adjust focused coverage if behavior changes. -
crates/buzz-db-backend-macro/src/lib.rs:11-12exports the newenforce_sqlite_backend_declarationsproc-macro entry point without a function-level doc comment. The crate-level overview does not document this public item or its attribute/marker contract. Add///API documentation as required byAGENTS.md:117.
Author action: Resolve both production panic-policy violations, document the exported proc macro, and resolve/rebase the currently conflicting branch before merge. Any new head requires delta review.
Verification owner: Author for the patch/rebase; reviewer for immutable-head delta inspection plus cargo test -p buzz-media and the macro trybuild suite. CI/integration environment owns Redis-, Postgres-, and MinIO-backed coverage unavailable locally.
Integrated review evidence
The team traced runtime-profile startup fencing, AppBackends injection, in-process pubsub/presence/control and admission limiting, filesystem media traversal/atomic writes/ranges/pagination/sidecars, unsupported search, Git-route suppression, SQLite compile-time selection, and product/UI scope. No client UI delta was found; deliberately rejecting BUZZ_PROFILE=single-node in PR 0/12 is sound scope fencing.
Exact-head local passes reported from clean worktrees:
cargo test -p buzz-db-backend-macro— pass, including trybuildcargo test -p buzz-pubsub— 27 passed, 11 Redis-dependent ignoredcargo test -p buzz-media— 111 passed, 1 live-MinIO ignoredcargo test -p buzz-search— 3 passed, 19 Postgres-dependent ignored
GitHub exact-head checks are green, including Unit Tests, Rust Lint, Backend Integration, Relay E2E, Security, cross-compiles, and DCO. Those are complementary evidence, not exceptions to the repository rules above.
Integration/confidence gaps
GitHub currently reports mergeable: CONFLICTING / mergeStateStatus: DIRTY; conflict resolution will move the immutable review target and needs complete delta/gate review. Local Redis, Postgres search, and live MinIO cases remained ignored due to unavailable services. These are integration/environment gaps rather than additional observed seam regressions.
2 participants
PR 0 of 12 — Backend seams prep (local-mode graduation stack)
First PR of the local-mode graduation train (source branch
wip-experiment-local-mode@b3f365faf, re-cut as a 12-PR stack; plan in thebuzz-experiment-local-modechannel canvas).What this PR does
Mechanical seams-prep refactor with zero hosted-path behavior change:
buzz-db-backend-macrocrate (new, standalone): compile-time enforcement that every publicDbmethod carries an explicit#[sqlite_backend(...)]decision marker, with trybuild compile-pass/fail coverage. No SQLite implementation code — the macro only enforces future declarations.RelayProfileconfig enum (BUZZ_PROFILE, defaults toproduction;single-nodefails fast at boot — constructor fence, no local backend installed yet),AppState::new_with_backendsinjection point,redis_poolbecomesOption(alwaysSomeon the production path), pubsub/media/search backend selection moved behind startup-selected wrappers.\\, drive/UNC paths), atomic same-directory temp-file writes/copies, seek + boundedtakerange reads, regression coverage for the traversal/Windows-separator boundary.Explicitly out of this PR (lands in PR 1 where first exercised): the broad
buzz-db/src/lib.rsbackend-operation gating, all SQLite persistence, sidecar/desktop work.Size
24 files, +1,414/−154 = 1,568 changed lines (stack budget: ≤2k after the PR 1 tracer).
Evidence
cargo test -p buzz-media— 111 passed / 1 ignored (MinIO, infra-dependent)cargo test -p buzz-db-backend-macro— green (incl. trybuild UI tests)cargo test -p buzz-pubsub— 27 passed / 11 ignoredcargo test -p buzz-db --lib— 94 passed / 154 ignoredcargo test -p buzz-relay --libat head vs plainorigin/main(2777189d9): identical 858 passed / 9 failed / 40 ignored — the 9 are pre-existing/environmental on the dev host (local Postgres container down: SQLx pool timeouts, admin 404→500, mesh-demo 504) and reproduce byte-for-byte at base. CI's real-database run is the authoritative gate.Review
Blocking review by Larry: initial 6/10 (filesystem media boundary needed the source branch's hardening family), fix round closed it, final 10/10 at exactly this head — no remaining blockers or suggestions.
Builder: Brother Darryl. Verification: Wakko (independent test reruns at exact head
8f29ecb).